Skip to content

feat(policies): PolicyEvaluation richer decision (reason + matchedRules)#20

Merged
jlc488 merged 1 commit into
mainfrom
feat/policy-evaluation-richer-decision
May 30, 2026
Merged

feat(policies): PolicyEvaluation richer decision (reason + matchedRules)#20
jlc488 merged 1 commit into
mainfrom
feat/policy-evaluation-richer-decision

Conversation

@jlc488

@jlc488 jlc488 commented May 27, 2026

Copy link
Copy Markdown
Contributor

Follow-up #2/5. #16 added wire-shape fields reason and matchedRules to PolicyTestResponse but had to leave them as nulls / empty lists because the Policy SPI only returned a PolicyDecision enum. This PR promotes the SPI so policies can actually surface detail.

SPI additions (access-api)

  • PolicyEvaluation record: { decision, reason, matchedRules }. Factory methods permit(reason, rules) / deny(reason, rules) / notApplicable(reason) / of(decision) keep call-sites readable and matchedRules immutable + non-null.
  • Policy.evaluateDetailed(context) — new default method. The default delegates to evaluate(context) so legacy policies that only implement the coarse entry point still produce a valid PolicyEvaluation (null reason + empty matched rules). Override evaluateDetailed directly when the policy has detail worth surfacing.
  • PolicyEvaluator.evaluateDetailed(name, context) — matching default. Delegates to evaluate by default; DefaultPolicyEvaluator overrides it to dispatch through Policy.evaluateDetailed.

Evaluator (access-core)

  • DefaultPolicyEvaluator.evaluateDetailed:
    • policyName == nullPolicyEvaluation.notApplicable("policyName was null")
    • unknown name → PolicyEvaluation.notApplicable("no policy registered for name: " + name)
    • match → policy.evaluateDetailed(context)

Controller (admin-api)

  • PolicyAdminController.test now calls evaluator.evaluateDetailed and forwards decision / reason / matchedRules into PolicyTestResponse. Wire shape unchanged — policies that don't override evaluateDetailed still respond cleanly with null reason + empty list.
  • Javadoc on PolicyTestResponse updated to explain the new semantics (no longer "placeholders today").

Backward compatibility

  • Every existing Policy implementation continues to compile and behave identically — they only have to override evaluate, and evaluateDetailed wraps that result automatically.
  • PolicyEvaluator.evaluate is unchanged, so callers that don't care about reason / matchedRules keep working.

Test plan

  • ./gradlew build green (55 tasks, all tests pass — DefaultPolicyEvaluatorTest didn't need any change because its policies override evaluate)
  • Manual: implement a policy that overrides evaluateDetailed to return permit("matched-role-admin", List.of("rule-role-admin")), hit /policies/test in the admin UI, verify the reason + matched rules render in the dialog

Follow-up #2/5 to the contract-gap series. #16 added the wire-shape
fields `reason` and `matchedRules` but had to leave them as nulls /
empty lists because the Policy SPI only returned a PolicyDecision
enum. This PR promotes the SPI so policies can actually surface a
human-readable reason and the list of rules that fired.

SPI additions (access-api)
--------------------------
- New PolicyEvaluation record: {decision, reason, matchedRules}.
  Constructed via factory methods (permit/deny/notApplicable/of)
  so call-sites stay readable and matchedRules is always non-null
  + immutable.
- Policy interface: new `default PolicyEvaluation evaluateDetailed(
  PolicyContext)`. The default wraps `evaluate(context)` so legacy
  policies that only implement the coarse entry point still
  produce a valid PolicyEvaluation (null reason + empty
  matchedRules). Override evaluateDetailed directly when the
  policy has detail worth surfacing.
- PolicyEvaluator interface: matching `default PolicyEvaluation
  evaluateDetailed(name, context)`. Default delegates to the
  coarse evaluate; DefaultPolicyEvaluator overrides it to dispatch
  through Policy.evaluateDetailed instead.

Evaluator (access-core)
-----------------------
- DefaultPolicyEvaluator.evaluateDetailed:
    * policyName == null     -> PolicyEvaluation.notApplicable(reason)
    * unknown policyName     -> PolicyEvaluation.notApplicable(reason)
    * matched policy         -> policy.evaluateDetailed(context)

Controller (admin-api)
----------------------
- PolicyAdminController.test now calls evaluator.evaluateDetailed
  and forwards the result's decision / reason / matchedRules into
  PolicyTestResponse. Policies that don't override evaluateDetailed
  still respond cleanly (null reason + empty list) — the wire shape
  is unchanged.
- Javadoc on PolicyTestResponse updated to explain the new
  reason / matchedRules semantics.

Backward compatibility
----------------------
- Every existing Policy implementation continues to compile and
  behave identically — they only have to override `evaluate`, and
  evaluateDetailed wraps that result automatically.
- DefaultPolicyEvaluator.evaluate is unchanged so PolicyEvaluator
  callers that don't care about reason / matchedRules keep working.
@jlc488
jlc488 merged commit 293f228 into main May 30, 2026
1 check passed
@jlc488
jlc488 deleted the feat/policy-evaluation-richer-decision branch May 30, 2026 06:37
jlc488 added a commit that referenced this pull request Jun 3, 2026
All implementation PRs have merged: export/import (#53), prod fail-fast
gating (#54), mirror mode + opt-in user sync (#55), and the admin-ui
"Config Sync" page (devslab-kit-admin-ui #20). Flip the status to Accepted
(item 6 of the plan) and record the PRs; the standalone how-to guide is
the remaining piece. Both language files.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant